Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

src/backend/core/api/viewsets.py (1)

2783-2803: ⚠️ Potential issue | 🔴 Critical

TOCTOU race between the existence/count check and get_or_create.

Two concurrent POSTs with different new emojis can both observe count() < REACTIONS_MAX_PER_COMMENT and both succeed, pushing the comment above the configured cap. Since the whole point of this PR is backend enforcement, the check + create should be serialized via transaction.atomic() + select_for_update() on the parent comment (as already proposed in an earlier review). This comment has already been raised on a prior commit.

Additionally, once you're inside the atomic block, you can reuse the prefetched comment.reactions.all() via len(...) instead of a second COUNT query.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/backend/core/api/viewsets.py` around lines 2783 - 2803, Wrap the
existence/count check and the get_or_create call in a single transaction and
lock the parent comment row to prevent TOCTOU races: start a
transaction.atomic() and re-fetch the Comment with select_for_update() (use the
same Comment instance used by the view), then check the number of distinct
reactions using the prefetched comment.reactions.all() (use len(...) instead of
.count()) and perform Reaction.objects.get_or_create(...) inside that atomic
block; update the code paths around the POST handling in the viewset so the
existence check and creation are serialized on the locked comment to enforce
settings.REACTIONS_MAX_PER_COMMENT.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/useComments.ts (1)

31-31: ⚠️ Potential issue | 🟡 Minor

Don't fall back to 0 before config loads.

When /config is still unresolved, or docs_config was cached before this field existed, 0 disables every new distinct emoji until the refetch completes. Use the backend default (15) or keep the picker disabled until config is ready.

Suggested fix

-        config?.REACTIONS_MAX_PER_COMMENT ?? 0,
+        config?.REACTIONS_MAX_PER_COMMENT ?? 15,

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/useComments.ts`
at line 31, The fallback to 0 for config?.REACTIONS_MAX_PER_COMMENT is disabling
emoji usage before config loads; change the logic in useComments (the place
using config?.REACTIONS_MAX_PER_COMMENT) to avoid returning 0 as the default:
either fall back to the backend default of 15 (e.g., use
config?.REACTIONS_MAX_PER_COMMENT ?? 15) or gate the picker until config is
ready (return undefined/null or a disabled state while config is unresolved),
ensuring REACTIONS_MAX_PER_COMMENT is not forced to 0 before the real config is
available.

src/backend/core/tests/documents/test_api_documents_comments.py (1)

881-901: ⚠️ Potential issue | 🟡 Minor

Pin the reaction cap inside the test.

The docstring says this test sets the limit to 10, but it still inherits whatever settings.REACTIONS_MAX_PER_COMMENT is at runtime. Overriding it via the fixture keeps the test deterministic and shorter.

Suggested fix

 def test_create_reaction_exceeds_maximum(settings):
     """
     Users should not be able to add more than REACTIONS_MAX_PER_COMMENT
-    (here we set it to 10) distinct emoji reactions to a comment.
+    distinct emoji reactions to a comment.
     They should, however, be able to add themselves to an existing reaction.
     """
+    settings.REACTIONS_MAX_PER_COMMENT = 10
+
     user1 = factories.UserFactory()
     user2 = factories.UserFactory()
@@
-    max_reactions = settings.REACTIONS_MAX_PER_COMMENT
+    max_reactions = settings.REACTIONS_MAX_PER_COMMENT

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/backend/core/tests/documents/test_api_documents_comments.py` around lines
881 - 901, The test test_create_reaction_exceeds_maximum relies on
settings.REACTIONS_MAX_PER_COMMENT at runtime; pin this cap inside the test to
make it deterministic by setting a local constant (e.g. MAX_REACTIONS = 10) and
use that value when generating emojis and asserting behavior instead of reading
settings.REACTIONS_MAX_PER_COMMENT, or explicitly override the Django setting
for the test (e.g. with monkeypatch or override_settings) to set
REACTIONS_MAX_PER_COMMENT = 10 before calling
factories.ReactionFactory.generate_emojis and the rest of the test logic so the
test always runs with the intended limit.

src/backend/impress/settings.py (1)

193-197: ⚠️ Potential issue | 🟡 Minor

Reject non-positive REACTIONS_MAX_PER_COMMENT at startup.

0 or negative values silently disable every new distinct reaction in both the API and the UI. Please fail fast with a clear configuration error instead of changing behavior at runtime.

Suggested guard

     def post_setup(cls):
         """Post setup configuration.
         This is the place where you can configure settings that require other
         settings to be loaded.
         """
         super().post_setup()
+
+        if cls.REACTIONS_MAX_PER_COMMENT < 1:
+            raise ValueError(
+                "REACTIONS_MAX_PER_COMMENT must be >= 1 "
+                "(env: REACTIONS_MAX_PER_COMMENT)."
+            )
 
         # The SENTRY_DSN setting should be available to activate sentry for an environment
         if cls.SENTRY_DSN is not None:

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/backend/impress/settings.py` around lines 193 - 197, The
REACTIONS_MAX_PER_COMMENT setting currently allows 0 or negative values which
silently disable reactions; add a startup validation right after the
REACTIONS_MAX_PER_COMMENT definition that checks its resolved integer is > 0 and
raises an explicit configuration error if not (e.g. raise
django.core.exceptions.ImproperlyConfigured or ValueError) with a clear message
referencing REACTIONS_MAX_PER_COMMENT and the environment variable
REACTIONS_MAX_PER_COMMENT (environ_name) so misconfiguration fails fast.

src/backend/impress/settings.py (1)

193-197: ⚠️ Potential issue | 🟡 Minor

Fail fast on invalid reaction limits.

Line 193-Line 197 currently allows REACTIONS_MAX_PER_COMMENT <= 0. A bad env value can silently disable new reactions. Validate at startup and raise a configuration error early.

Suggested fix

 class Base(Configuration):
@@
     def post_setup(cls):
@@
         super().post_setup()
+        if cls.REACTIONS_MAX_PER_COMMENT < 1:
+            raise ValueError(
+                "REACTIONS_MAX_PER_COMMENT must be >= 1 (env: REACTIONS_MAX_PER_COMMENT)."
+            )

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/backend/impress/settings.py` around lines 193 - 197,
REACTIONS_MAX_PER_COMMENT currently permits non-positive values from the
environment which can silently disable reactions; add a startup validation in
settings.py to check that REACTIONS_MAX_PER_COMMENT > 0 (after the IntegerValue
is resolved) and raise a clear configuration error (e.g., ValueError or Django's
ImproperlyConfigured) if the value is <= 0 so invalid env settings fail fast;
update the settings initialization logic around the REACTIONS_MAX_PER_COMMENT
symbol to perform this check and raise with a descriptive message.

src/backend/core/tests/documents/test_api_documents_comments.py (1)

940-959: ⚠️ Potential issue | 🟡 Minor

Align test setup with its documented behavior.

Line 942 says “here we set it to 10”, but the test never overrides the setting; it uses current runtime config instead.

Suggested fix

 def test_create_reaction_exceeds_maximum(settings):
@@
-    user1 = factories.UserFactory()
+    settings.REACTIONS_MAX_PER_COMMENT = 10
+    user1 = factories.UserFactory()
@@
-    max_reactions = settings.REACTIONS_MAX_PER_COMMENT
+    max_reactions = settings.REACTIONS_MAX_PER_COMMENT

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/backend/core/tests/documents/test_api_documents_comments.py` around lines
940 - 959, The test claims "here we set it to 10" but reads
settings.REACTIONS_MAX_PER_COMMENT at runtime; fix by explicitly setting the
limit to 10 for the test (e.g., use Django's override_settings or monkeypatch to
set settings.REACTIONS_MAX_PER_COMMENT = 10) before computing max_reactions and
calling factories.ReactionFactory.generate_emojis; ensure the code that
references max_reactions (the variable max_reactions and the call to
factories.ReactionFactory.generate_emojis) uses this overridden value so the
test matches its docstring.

src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/useComments.ts (1)

31-31: ⚠️ Potential issue | 🟠 Major

Avoid 0 as fallback for max reactions.

Line 31 (config?.REACTIONS_MAX_PER_COMMENT ?? 0) temporarily disables adding new distinct reactions before config resolves (or when cached config predates this field).

Suggested fix

-        config?.REACTIONS_MAX_PER_COMMENT ?? 0,
+        config?.REACTIONS_MAX_PER_COMMENT ?? 15,

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/useComments.ts`
at line 31, The current fallback of 0 for config?.REACTIONS_MAX_PER_COMMENT
prevents adding reactions until config loads; in useComments.ts replace the
fallback (the expression using REACTIONS_MAX_PER_COMMENT) with a permissive
default so reactions remain allowed (e.g., use config?.REACTIONS_MAX_PER_COMMENT
?? Number.POSITIVE_INFINITY or a sensible constant like DEFAULT_MAX_REACTIONS >
0) and ensure any UI checks treat Infinity/undefined as "no enforced limit".
Update references to REACTIONS_MAX_PER_COMMENT in the useComments hook to use
that new default value.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/useComments.ts (2)

8-8: ⚠️ Potential issue | 🟡 Minor

Reorder imports to match the frontend lint rule.

Line 8 reintroduces the alias-ordering issue (@/core should be grouped before @/cunningham).

🧹 Suggested reorder

+import { useConfig } from '@/core';
 import { useCunninghamTheme } from '@/cunningham';
 import { User, avatarUrlFromName } from '@/features/auth';
 import { useEditorStore } from '@/features/docs/doc-editor/stores';
 import { Doc, useProviderStore } from '@/features/docs/doc-management';
-import { useConfig } from '@/core';

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/useComments.ts`
at line 8, Reorder the import groups in useComments.ts so alias '@/core' appears
before '@/cunningham' to satisfy the frontend alias-ordering lint rule;
specifically move or reorder the line importing useConfig (import { useConfig }
from '@/core') so it is grouped prior to any imports from '@/cunningham' (and
ensure import grouping/ordering for other aliases follows the same rule).

---

31-31: ⚠️ Potential issue | 🟡 Minor

Use a non-zero fallback for maxReactions while config is loading.

Line 31 falls back to 0, which temporarily disables adding new distinct reactions before config resolves.

💡 Suggested fix

-        config?.REACTIONS_MAX_PER_COMMENT ?? 0,
+        config?.REACTIONS_MAX_PER_COMMENT ?? 15,

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/frontend/apps/impress/src/features/docs/doc-editor/components/comments/useComments.ts`
at line 31, The fallback for config?.REACTIONS_MAX_PER_COMMENT currently uses 0
which blocks adding reactions while config loads; in useComments.ts change the
fallback to a sensible non-zero default (e.g., DEFAULT_MAX_REACTIONS = 8) and
use that instead of 0 for maxReactions (referencing
config?.REACTIONS_MAX_PER_COMMENT and the maxReactions variable) so distinct
reactions remain enabled until the real config resolves.

src/backend/core/tests/documents/test_api_documents_comments.py (1)

940-943: ⚠️ Potential issue | 🟡 Minor

Make the test setup match the stated scenario (“set to 10”).

Lines 940-943 state a fixed limit of 10, but the test currently uses whatever default is configured at runtime.

✅ Suggested fix

 def test_create_reaction_exceeds_maximum(settings):
     """
     Users should not be able to add more than REACTIONS_MAX_PER_COMMENT
     (here we set it to 10) distinct emoji reactions to a comment.
     They should, however, be able to add themselves to an existing reaction.
     """
+    settings.REACTIONS_MAX_PER_COMMENT = 10
     user1 = factories.UserFactory()
     user2 = factories.UserFactory()
@@
-    max_reactions = settings.REACTIONS_MAX_PER_COMMENT
+    max_reactions = settings.REACTIONS_MAX_PER_COMMENT

Also applies to: 958-959

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/backend/core/tests/documents/test_api_documents_comments.py` around lines
940 - 943, The test asserts a fixed limit of 10 reactions but currently uses the
runtime default; explicitly set the limit to 10 in the test setup by patching
the constant or configuration used by the code under test (e.g., monkeypatch
core.constants.REACTIONS_MAX_PER_COMMENT = 10 or override the app setting the
code reads) before creating reactions, and apply the same explicit override in
the related assertions (the blocks around REACTIONS_MAX_PER_COMMENT usage at the
other affected lines) so the test environment matches the stated scenario.

src/backend/impress/settings.py (1)

193-197: ⚠️ Potential issue | 🟠 Major

Fail fast on invalid REACTIONS_MAX_PER_COMMENT values.

Lines 193-197 accept invalid values (e.g., 0/negative), which silently changes API/UI behavior. Add a startup guard in post_setup to enforce >= 1.

🛡️ Suggested guard

 def post_setup(cls):
     """Post setup configuration.
     This is the place where you can configure settings that require other
     settings to be loaded.
     """
     super().post_setup()
+
+    if cls.REACTIONS_MAX_PER_COMMENT < 1:
+        raise ValueError(
+            "REACTIONS_MAX_PER_COMMENT must be >= 1 (env: REACTIONS_MAX_PER_COMMENT)."
+        )
 
     # The SENTRY_DSN setting should be available to activate sentry for an environment
     if cls.SENTRY_DSN is not None:

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/backend/impress/settings.py` around lines 193 - 197, Add a startup guard
in the settings module's post_setup to validate REACTIONS_MAX_PER_COMMENT: read
the configured REACTIONS_MAX_PER_COMMENT value, and if it is less than 1 raise a
clear startup exception (e.g., ValueError or ImproperlyConfigured) with a
message explaining the invalid value and required minimum; reference the
REACTIONS_MAX_PER_COMMENT symbol and implement this check inside the existing
post_setup function so the app fails fast on 0/negative values.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes